Skip to content

[Fix] multiple git remotes (CF-677) #542

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom
Merged
merged 5 commits into from
Jul 14, 2025
Merged

[Fix] multiple git remotes (CF-677) #542

merged 5 commits into from
Jul 14, 2025

Conversation

@mohammedahmed18
Copy link
Contributor

@mohammedahmed18 mohammedahmed18 commented Jul 12, 2025 •
edited by github-actions bot
Loading

User description

Tested this with two remotes: origin and fork.
origin points to this repository, while fork refers to this fork.
By setting the git-remote arg to fork, it raised this PR within the forked repository.

PR Type

Enhancement, Tests

Description

  • Introduce configurable git_remote parameter

  • Update CLI to pass git_remote arg

  • Adjust PR creation to use git_remote

  • Revise tests for git_remote support

Changes walkthrough 📝

Relevant files
Enhancement
cli.py
Include git_remote in CLI branch check                                     

codeflash/cli_cmds/cli.py

  • Pass args.git_remote to check_and_push_branch
 +1/-1     
git_utils.py
Make push remote configurable via git_remote                         

codeflash/code_utils/git_utils.py

  • Add git_remote parameter to function signature
  • Replace hardcoded origin with git_remote
  • Update push and logging to use git_remote
    		• +6/-6     
    create_pr.py
    Use git_remote uniformly in PR creation                                   

    codeflash/result/create_pr.py

  • Pass git_remote to get_repo_owner_and_name
  • Use git_remote in check_and_push_branch call
  • Update PR creation logs to include git_remote
    		• +1/-1     
    Tests
    test_git_utils.py
    Update tests for git_remote parameter                                       

    tests/test_git_utils.py

  • Add git_remote argument to tests
  • Adjust assertions for remote origin usage
    		• +3/-3     
    Need help?
  • Type /help how to ... in the comments thread for any questions about PR-Agent usage.
  • Check out the documentation for more information.
    •

    @github-actions
    Copy link

    PR Reviewer Guide 🔍

    Here are some key observations to aid the review process:

    ⏱️ Estimated effort to review: 2 🔵🔵⚪⚪⚪
    🧪 PR contains tests
    🔒 No security concerns identified
    ⚡ Recommended focus areas for review

    Missing Argument

    Verify that args.git_remote is always defined with a default in the CLI parser to prevent AttributeError when accessed in handle_optimize_all_arg_parsing.

     
    if not args.no_pr and not check_and_push_branch(git_repo, git_remote=args.git_remote):
    Inconsistent API

    Ensure get_repo_owner_and_name supports both one- and two-argument calls, or update all invocations to pass git_remote, to avoid breaking existing CLI flows.

     
    owner, repo = get_repo_owner_and_name(git_repo, git_remote)
logger.info(f"Pushing to {git_remote} - Owner: {owner}, Repo: {repo}")
    Ref Comparison

    repo.refs is a list of Reference objects, not strings; comparing f"{git_remote}/{current_branch}" directly may never match. Consider comparing ref.name attributes or using repo.refs.by_name.

     
    # Check if the branch is pushed
if f"{git_remote}/{current_branch}" not in repo.refs:
    logger.warning(f"⚠️ The branch '{current_branch}' is not pushed to the remote repository.")

    @github-actions
    Copy link

    PR Code Suggestions ✨

    Explore these optional code suggestions:

    CategorySuggestion                                                                                                                                    Impact
    Possible issue
    Strict remote ref check

    Use an explicit name comparison against repo.refs to avoid mismatches, ensuring the
    branch existence check is accurate.

    codeflash/code_utils/git_utils.py [125]

    -if f"{git_remote}/{current_branch}" not in repo.refs:
+if not any(ref.name == f"{git_remote}/{current_branch}" for ref in repo.refs):
    Suggestion importance[1-10]: 6

    __

    Why: Switching to an explicit ref.name comparison prevents subtle membership mismatches in repo.refs, improving correctness of the branch existence check.

    		Low
    Handle missing remote exception

    Wrap the remote lookup in a try/except to catch and handle missing remote names
    gracefully, falling back to "origin" if needed.

    codeflash/code_utils/git_utils.py [122]

    -remote = repo.remote(name=git_remote)
+try:
+    remote = repo.remote(name=git_remote)
+except git.exc.NoSuchRemoteException:
+    logger.error(f"No such remote '{git_remote}', falling back to 'origin'.")
+    remote = repo.remote(name="origin")
    Suggestion importance[1-10]: 5

    __

    Why: Catching a potential NoSuchRemoteException enhances resilience when the provided remote doesn’t exist, although it’s a non-critical improvement.

    		Low
    General
    Provide default remote parameter

    Add a default value for git_remote to preserve backward compatibility and simplify
    calls when no remote is specified.

    codeflash/code_utils/git_utils.py [120]

    -def check_and_push_branch(repo: git.Repo, git_remote: str, wait_for_push: bool = False) -> bool:  # noqa: FBT001, FBT002
+def check_and_push_branch(repo: git.Repo, git_remote: str = "origin", wait_for_push: bool = False) -> bool:  # noqa: FBT001, FBT002
    Suggestion importance[1-10]: 4

    __

    Why: Adding a default value for git_remote helps maintain backward compatibility and simplifies calls, but it’s a minor API convenience rather than a critical fix.

    		Low
    Default to origin if missing

    Safely access args.git_remote with a default to avoid attribute errors when the flag
    isn’t provided.

    codeflash/cli_cmds/cli.py [237]

    -if not args.no_pr and not check_and_push_branch(git_repo, git_remote=args.git_remote):
+remote_name = getattr(args, "git_remote", "origin")
+if not args.no_pr and not check_and_push_branch(git_repo, git_remote=remote_name):
    Suggestion importance[1-10]: 3

    __

    Why: Safely defaulting args.git_remote guards against missing attributes, but the CLI likely defines this argument already and the impact is limited.

    		Low

    @mohammedahmed18 mohammedahmed18 requested a review from KRRT7 July 12, 2025 16:29
    @misrasaurabh1 misrasaurabh1 merged commit 88b79ea into main Jul 14, 2025
    15 of 16 checks passed
    Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

    Reviewers

    @misrasaurabh1 misrasaurabh1 misrasaurabh1 approved these changes

    @KRRT7 KRRT7 Awaiting requested review from KRRT7

    Assignees

    No one assigned

    Labels

    Review effort 2/5

    Projects

    None yet

    Milestone

    No milestone

    Development

    Successfully merging this pull request may close these issues.

    2 participants

    @mohammedahmed18 @misrasaurabh1